Introduce cpplint check #334
Conversation
mathieucarbou
force-pushed
the
cpplint
branch
3 times, most recently
from
d744587 to
97749fc
Compare
|
it removed all |
mathieucarbou
force-pushed
the
cpplint
branch
from
97749fc to
6673954
Compare
for explicit, this is my mistakes. Also, I disabled the explicit check: About override, this is not necessary when final is there. If final is there it implicitly means override. cpplint justs check: I did the cleaup. |
|
FYI, please review / approve but please do not merge yet: I would prefer to do the release first and merge after. Thanks! |
|
OK, if it's only with |
| #include <memory> | ||
| #include <vector> | ||
|
|
||
| #include "./literals.h" |
There was a problem hiding this comment.
why relative? chance to have another header with the same name elsewhere?
There was a problem hiding this comment.
no - that's just a cpplint request to have path indications when using "..." instead of <...> in order to have the includes formatted the same way everywhere. I think this one is ok, but we can disable this check if we want.
There was a problem hiding this comment.
it's not the same though. Well it will point to the same file, but not in the same way. "literals.h" is file in one of the include paths, while "./literals.h" means file in the same folder as the file which includes it that way
There was a problem hiding this comment.
yes I know the difference...
I think cpplint uses this programming convention:
"./path/to/include.h"<from/compile/path/include/h>
They "ask" devs to follow this convention (if we keep the flag).
I am personally not against this convention since it adds more clarity when reading the code.
Currently, this is a mess in our code base for the includes. There is no strict conventions.
There was a problem hiding this comment.
it will work either way in this case (relative or not), but just last few days we've been dealing with some relative paths in ESP-IDF too and they've proven to be messy. Anyway, I just wanted to know why did you opt for relative path. Otherwise, my understanding is that "header.h" is library/project local include, while <header.h> is something that comes from elsewhere (another library)
There was a problem hiding this comment.
Otherwise, my understanding is that "header.h" is library/project local include, while <header.h> is something that comes from elsewhere (another library)
Yes the rational comes from that.
And that's also the convention cpplint is enforcing with this check.
So if OK we can go with that for the project from now on, and maybe on day take the opportunity to cleanup the existing includes.
Wit hsome filters that are not impacting a lot the current code